-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[IR] Don't store switch case values as operands #166842
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
SwitchInst case values must be ConstantInt, which have no use list. Therefore it is not necessary to store these as Use, instead store them more efficiently as a simple array of pointers after the uses, similar to how PHINode stores basic blocks. After this change, the successors of all terminators are stores consecutively in the operand list. This is preparatory work for improving the performance of successor access. WIP: SandboxIR is broken.
|
That's a very interesting case @aengelke , thanks for sharing! Let's start with the obvious: the tests are crashing because of the nullptr in the line you added: The fix is quite simple. Similarly to most other components, Sandbox IR's CaseHandle (and most probably the case iterator too) should become its own separate class that depends on LLVM IR' public API and remove the dependency back to Sandbox IR. I will follow-up with a patch that fixes this. |
SandboxIR's SwitchInst CaseHandle was relying on LLVM IR's SwitchInst::CaseHandleImpl template, which may call private functions of SandboxIR's SwitchInst. This creates a dependency cycle which is against the design principles of Sandbox IR. The issue was exposed by: llvm#166842 Thanks to @aengelke for raising the issue.
SandboxIR's SwitchInst CaseHandle was relying on LLVM IR's SwitchInst::CaseHandleImpl template, which may call private functions of SandboxIR's SwitchInst. This creates a dependency cycle which is against the design principles of Sandbox IR. The issue was exposed by: llvm#166842 Thanks to @aengelke for raising the issue.
…167093) SandboxIR's SwitchInst CaseHandle was relying on LLVM IR's SwitchInst::CaseHandleImpl template, which may call private functions of SandboxIR's SwitchInst. This creates a dependency cycle which is against the design principles of Sandbox IR. The issue was exposed by: #166842 Thanks to @aengelke for raising the issue.
…CaseHandle (#167093) SandboxIR's SwitchInst CaseHandle was relying on LLVM IR's SwitchInst::CaseHandleImpl template, which may call private functions of SandboxIR's SwitchInst. This creates a dependency cycle which is against the design principles of Sandbox IR. The issue was exposed by: llvm/llvm-project#166842 Thanks to @aengelke for raising the issue.
SwitchInst case values must be ConstantInt, which have no use list. Therefore it is not necessary to store these as Use, instead store them more efficiently as a simple array of pointers after the uses, similar to how PHINode stores basic blocks.
After this change, the successors of all terminators are stores consecutively in the operand list. This is preparatory work for improving the performance of successor access.
WIP: SandboxIR is broken.
cc @vporpo for SandboxIR
cc @nikic
Other things to consider: